Pass through PHP notices, warnings, and errors to test harness and fix resulting failures #4074
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
error_handler now only runs set_error_handler in web mode. E_ALL notices, warnings, and errors are now reported, which causes the test harness to fail.
Removes CLI-invoked E_NOTICE in: * e107::prepare_request() * e107::set_constants() * e107::set_urls()
Resolves CLI-invoked E_NOTICE in: * eIPHandler::__construct() * eIPHandler::getCurrentIP() Also resolves possible blank eIPHandler::$serverIP
\Helper\Unit::_beforeSuite() now sets E107_DEBUG_LEVEL so that debug_handler.php sets the debug mode. Also fixed E_NOTICE if E107_DEBUG_LEVEL is set beforehand
Resolves CLI-invoked E_NOTICE in: * e_online::goOnline()
Resolves CLI-invoked E_NOTICE in: * e_session::getValidateData() * e_core_session::challenge()
- FIX: e107::coreLan() now loads the lan_admin.php file if the $admin argument is true - FIX: Variable scope of $eplug_folder in e107plugin::uninstall() - FIX: isset() check order in pluginsTest::makePluginReport() - FIX: class2.php: Missing ADMINPERMS constant in CLI mode
- FIX: Do not redefine e_ADMIN_AREA in parser.php - FIX: Null checks for e107TinyMceParser - FIX: Array type check for e_bbcode::imgToBBcode() - FIX: Optional query string in e_parse::thumbUrlDecode() - FIX: Don't redefine TINYMCE_UNIT_TEST
Was causing an undefined index error
Also suppress mkdir() error in e_file::unzipGithubArchive()
e_DEBUG is already set because of the new test debug strategy
- MOD: e107::getTemplate() now accepts blank strings for the plugin name to mean
that a core template should be loaded
- FIX: e_form::progressBar() now supports input values that already have % at the end
- FIX: Null check for $options['list'] in e_form::progressBar()
- NEW: Test rounding in e_formTest::testProgressBar()
Apparently e_marketplace depends on the admin area theme LAN
- FIX: Removed pointless (and invalid) destructor in LinkedIn::__destruct()
- FIX: All files that trigger this deprecation notice in PHP 7.4:
"Array and string offset access syntax with curly braces is deprecated"
Apparently a bug introduced on 2004-09-21: https://sourceforge.net/p/e107/svn/898/tree/trunk/e107_0.7/e107_languages/English/lan_ren_help.php Just happens to be a misuse of the third parameter of define()
- FIX: Silenced compact() in e107Test::testInitCore()
- FIX: Null check in e_db_pdo::makeTableDef()
- FIX: Null check in e_db_mysql::makeTableDef()
- FIX: userlogin::login() had this warning on line 148:
"continue" targeting switch is equivalent to "break". Did you mean to use "continue 2"?
- FIX: e_array::unserialize() HTML vomit in CLI mode - FIX: e107_debug_shutdown() HTML vomit because $error_handler was not global
Apparently requires the "gallery" plugin to be installed
Deltik
added
type: bug
- FIX: Don't redefine MYSQL_* constants in e_db_pdo_class.php
- FIX: e_db_mysql::rowCount() could try to use mysql_num_rows() to count rows of a non-resource
- FIX: e_db_mysql::delete() stores the number of deleted rows in e_db_mysql::$mySQLrows
- FIX: e_db_abstractTest::testDb_Query() was fetching in PDO mode but shouldn't have been
- FIX: Typos in e_db_abstractTest::testDelete()
- MOD: Moved PDO-exclusive testBackup() from e_db_abstractTest to e_db_pdoTest
- FIX: e_db_mysqlTest now works in PHP 5.6 if the main e_db instance was in PDO mode but the test
class initializes in legacy mode
- MOD: e_db_mysqlTest now asserts that PDO mode is not in use
- FIX: e_db_mysqlTest::testGetServerInfo() should now actually get a version number
- FIX: e_db_mysqlTest::testGetLastErrorNumber() has a different error code compared to PDO
- FIX: e_db_mysqlTest::testEscape() should actually get something from mysql_real_escape_string()
e_db_mysql has divorced e_db_pdo. They are now independently functioning implementations of e_db.
Finally no longer using the mysql_* functions removed in PHP 7.0
- MOD: Silently ignore failures to e_db_mysql::close(); if it's failing, it's probably already closed - FIX: Close the PDO or mysqli connection after each e_db_abstractTest test - MOD: Changed class2.php's $sql variable to be hinted as an e_db instead of e_db_mysql
codecept_output_dir() might not exist when the PHP-serialized coverage report is being generated. phpunit/php-code-coverage >= 6.0.8 fix this by creating that directory before writing the coverage report. PHP 5.6 can only get phpunit/php-code-coverage up to version 4.0.8, which does not have this fix. A workaround has been introduced in this commit to allow PHP-serialized coverage reports to be stored with PHP 5.6.
|
Code Climate has analyzed commit f2a7590 and detected 19 issues on this pull request. Here's the issue category breakdown:
Note: there is 1 critical issue. The test coverage on the diff in this pull request is 29.0% (80% is the threshold). This pull request will bring the total coverage in the repository to 5.8% (0.3% change). View more on Code Climate. |
Deltik
changed the title
|
@Deltik Could you tell me more about getTemplate() now loading core templates.. while we already have getCoreTemplate() ? |
|
@Deltik lan_admin.php is no longer loaded early enough. We now have this on plugin admin pages: I have restored the early loading of lan_admin.php |
@CaMer0n: I made this change because of This then leads to And from there, That empty string is passed on as the first argument to
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation and Context
e107's workaround for the many PHP notices, warnings, and errors has been to suppress them. This is especially detrimental for PHP major version changes (forward compatibility) because deprecated behavior would not be discovered until the code breaks.
Other less critical factors are type mismatches, resulting in PHP notices that get ignored. These are code smells or signs of problematic code. These notices can be resolved by being explicit about the desired type and more clearly conveys the developer's intention of what the variable should be.
Description
Overview
error_reporting(E_ALL)for all supported versions of PHP (versions 5.6 through 7.4).mysqlextension withmysqliin thee_db_mysqlclassBreaking Changes
error_handler::handle_error())when
$_E107['cli']or$_E107['debug']is not false.$_E107['debug']mode, the error reporting level isE_ALL.$_E107['cli']mode, the error reporting level isE_ALL & ~E_STRICT & ~E_NOTICE.ALLOW_AUTO_FIELD_DEFSconstante107::coreLan()now loads thelan_admin.phpfile if the$adminargument is truee107::getTemplate()now accepts blank strings for the plugin name to mean that a core template should be loaded.e_db_mysqlUSE_PERSISTANT_DBconstante_db_mysql: Replacedmysqlwithmysqli. Return types have changed but no tests needed to be modified, so client code should remain stable.e_db_mysql::close(); if it's failing, it's probably already closedBug Fixes
mkdir()failure ine107::_init()if parent directory does not existeIPHandler::$serverIPin CLI modepclzip.lib.phpto version 2.8.4 to fix math errormkdir()error ine_file::unzipGithubArchive()e107::getTemplate()could be run without the expected plugin LANsLinkedIn::__destruct()lan_ren_help.phpcaused by this commit on 21 September 2004.userlogin::login()had acontinuethat should have been acontinue 2inside aswitch.e_array::unserialize()HTML vomit in CLI modee107_debug_shutdown()HTML vomit due to$error_handlernot being globale_db_mysql::rowCount()could try to usemysql_num_rows()to count rows of a non-resourcee_db_mysql::delete()now stores the number of deleted rows ine_db_mysql::$mySQLrowse_db_mysql::getServerInfo()should now actually get a version numberVariable and Type Corrections
$_SERVERkeys in:e107::prepare_request()e107::set_constants()e107::set_urls()$_SERVERkeys in:eIPHandler::__construct()eIPHandler::getCurrentIP()$_SERVERkeys in:e_online::goOnline()$_SERVERkeys in:e_session::getValidateData()e_core_session::challenge()$eplug_folderine107plugin::uninstall()array_diff_recursive()type check during recursione_form::option_multi()e_db::insert()implementationse_bbcode::imgToBBcode()e_parse::thumbUrlDecode()e107TinyMceParsere107plugin::XmlAdminLinks()e107plugin::XmlTables()db_verify::prepareResults()online_shortcodes::sc_online_member_user()UserHandler::userClassUpdate()e_file::file_size_decode()e_form::progressBar()now supports input values that already have%at the end$options['list']ine_form::progressBar()e_db_pdo::db_Query()e107Test::testGetTemplate()e_themeconstructore_user_model::isBot()e_tree_model::flattenTree()e_tohtml_linkwords::linksproc()navigation_shortcode()e107_core/bbcodes/link.bbe_parse::thumbSrcSet()e_db_pdo::makeTableDef()e_db_mysql::makeTableDef()e107::url()class2.php's$sqlvariable to be hinted as ane_dbinstead ofe_db_mysqlConstant Definitions
class2.php: MissingADMINPERMSconstant in CLI modee_marketplace. Apparentlye_marketplacedepends on the admin area theme LANEUF_*constants in thee107_user_extendedconstructorMYSQL_*constants ine_db_pdo_class.phpTest Harness Changes
isset()check order inpluginsTest::makePluginReport()e_onlineTest::testGoOnline()because it has no assertionsuser_classTest::testGetUsersInClass()TINYMCE_UNIT_TESTe_DEBUGine_arrayTest::testUnserialize()e_db_abstractTest::testDb_Fetch()e_formTest::testProgressBar()e_db_abstractTest:testDb_Mark_Time()needed the debug log to be resettestDb_IsLang()needed the admin LAN to be loadedpluginsTestdb_verifyTest::testGetIndex()because it has no assertionse107Test::testGetInstance()includede107_config.phptoo many timesTreeModelTest::testTreeParentsAreAssignedCorrectly()apparently never worked until now because the wrong index was usede_formTest::testRenderElement()compact()ine107Test::testInitCore()e_pluginTest::testBuildAddonPrefList()due to the "gallery" plugin possibly being uninstallede_db_abstractTest::testDb_Query()was fetching in PDO mode but shouldn't have beene_db_abstractTest::testDelete()testBackup()frome_db_abstractTesttoe_db_pdoTeste_db_mysqlTestnow works in PHP 5.6 if the maine_dbinstance was in PDO mode but the test class initializes in legacy modee_db_mysqlTestnow asserts that PDO mode is not in usee_db_mysqlTest::testGetLastErrorNumber()has a different error code compared to PDO. This is now accounted for.e_db_mysqlTest::testEscape()should actually be escaped bymysql_real_escape_string()mysqliextension so thate_db_mysqlcan be tested.e_db_abstractTesttest.mkdir(). Fixes test coverage generation in PHP 5.6.How Has This Been Tested?
See "Test Harness Changes" above.
Types of Changes
Checklist